Show arguments names in tooltips for local functions #13429
Conversation
|
@psfinaki Could you also check if it fixes |
| module Test = | ||
| type T() = | ||
| let fu$$nc x = () | ||
| """ |
There was a problem hiding this comment.
Could you also add a test for bindings that are local to other expressions, not types?
do
let func x = ()
()There was a problem hiding this comment.
Actually looks like this case is not working yet, hah. Looking into that
@auduchinok Sooo likely yes, looks like that |
|
So apparently we already had tests verifying the wrong behavior so will do something about those as well :D |
I think this should be done in a separate PR. I suspect it's considerably harder In particular I believe we can't set ValReprInfo for Val contained in expressions without mucking with later compilation (e.g. confusing IlxGen or later phases) Note the class-bound Val being adjusted in this PR are ephemeral nodes used during type checking that are not actually present in the TypedTree (they get replaced by new member Vals), and are not relevant to later compilation. The expression-bound Val are not like this - they are present in the typed tree. Possible solutions:
|
|
So I fixed the tests, updated the description, @auduchinok we'll address the other broken case separately, I'm all up for smaller PRs :) |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Great to have this case fixed! |
This sounds like the best option.
And this one is good enough if it's considerably easier to do than the previous one. |
partially fixes #7080
So basically local functions were missing value representation info which the tooltips are relying on. This is fixed by calling
AdjustValToTopValfunction which sets this value representation (akatop val info) in the right place.Note that this doesn't fix the other case described in the issue, see @dsyme's comment.
